Repository navigation
Fix particleset repr - #3021
Fix particleset repr#3021erikvansebille wants to merge 3 commits into
Conversation
Improving readability and extending unit tests
| def __getitem__(self, index): | ||
| """Get a single particle by index.""" | ||
| return ParticleSetView(self._data, index=index, pclass=self._pclass) | ||
| return ParticleSetView(self._data, index=index, pclass=self._pclass, time_interval=self.fieldset.time_interval) |
There was a problem hiding this comment.
This is needed in order to print full dates (using parcels.float_to_datelike) for e.g. print(pset[0]). If we don't add the time-Interval, these Particle.t could only be printed as floats.
Do you agree the date-printing is worth the extra complexity of adding another argument to ParticleSetView.__init__?
There was a problem hiding this comment.
I think injecting FieldSet context into ParticlesetView like this weakens the abstraction of the ParticleSetView (and also is misleading to users, since the data stored in ParticleSetView are floats). Personally I don't think its worth it
| time_string = "not_yet_set" if pview.t is None or np.isnan(pview.t) else f"{pview.t:f}" | ||
| out = f"P[{pview.particle_id}]: time={time_string}, z={pview.z:f}, y={pview.y:f}, x={pview.x:f}" | ||
| vars = [v.name for v in pview._pclass.variables if v.to_write is True and v.name not in ["z", "y", "x", "time"]] | ||
| from parcels._core.utils.time import float_to_datelike |
There was a problem hiding this comment.
Need to import here because otherwise a circular import?
E ImportError: cannot import name 'float_to_datelike' from partially initialized module 'parcels._core.utils.time' (most likely due to a circular import) (/Users/erik/Codes/parcels/src/parcels/_core/utils/time.py)
| """Return a pretty repr for Variable""" | ||
| return f"Variable(name={var._name!r}, dtype={var.dtype!r}, initial={var.initial!r}, to_write={var.to_write!r}, attrs={var.attrs!r})" | ||
| out = f"Variable(name={var._name!r}, dtype={np.dtype(var.dtype).name!r}, initial={var.initial!r}, to_write={var.to_write!r}" | ||
| if getattr(var, "attrs", None): |
There was a problem hiding this comment.
Don't print empty attrs dictionaries
There was a problem hiding this comment.
I understand the change on this line about the dtype , but why shouldn't the empty attrs dict be printed? I think showing it is better
| """Return a pretty repr for Variable""" | ||
| return f"Variable(name={var._name!r}, dtype={var.dtype!r}, initial={var.initial!r}, to_write={var.to_write!r}, attrs={var.attrs!r})" | ||
| out = f"Variable(name={var._name!r}, dtype={np.dtype(var.dtype).name!r}, initial={var.initial!r}, to_write={var.to_write!r}" | ||
| if getattr(var, "attrs", None): |
There was a problem hiding this comment.
I understand the change on this line about the dtype , but why shouldn't the empty attrs dict be printed? I think showing it is better
| time_string = "not_yet_set" if pview.t is None or np.isnan(pview.t) else f"{pview.t:f}" | ||
| out = f"P[{pview.particle_id}]: time={time_string}, z={pview.z:f}, y={pview.y:f}, x={pview.x:f}" | ||
| vars = [v.name for v in pview._pclass.variables if v.to_write is True and v.name not in ["z", "y", "x", "time"]] | ||
| from parcels._core.utils.time import float_to_datelike |
| def __init__(self, data, index, pclass): | ||
| def __init__(self, data, index, pclass, time_interval=None): | ||
| self._data = data | ||
| self._index = index | ||
| self._pclass = pclass | ||
| self._time_interval = time_interval |
There was a problem hiding this comment.
Why does this state need to be added? just saw the other comment
Description
This PR (re-)implements
ParticleSet.__repr__andParticleSetView.__repr__. It also adds tests and improves/cleans up formattingChecklist
mainfor normal development,v3-supportfor v3 support)AI Disclosure
CoPilot has helped me with some of the changes to the
ParticleSetView.__repr__